Skip to content

GH-50105: [C++][Python] Fix sliced sparse union null checks#50108

Merged
pitrou merged 1 commit into
apache:mainfrom
fenfeng9:issue-50105-sparse-union-element-access
Jun 10, 2026
Merged

GH-50105: [C++][Python] Fix sliced sparse union null checks#50108
pitrou merged 1 commit into
apache:mainfrom
fenfeng9:issue-50105-sparse-union-element-access

Conversation

@fenfeng9

@fenfeng9 fenfeng9 commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Rationale for this change

Sliced sparse unions could report incorrect nullness during element access.
The type id lookup used the union offset, but the child null check still used the slice-relative index.

What changes are included in this PR?

  • Updates IsNullSparseUnion and ArraySpan::IsNullSparseUnion in data.cc to use the parent offset when checking child nullness.
  • Add C++ and Python regression tests.

Are these changes tested?

Yes.

Are there any user-facing changes?

No.

@fenfeng9 fenfeng9 requested review from AlenkaF, raulcd and rok as code owners June 5, 2026 16:43
@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown

⚠️ GitHub issue #50105 has been automatically assigned in GitHub to PR creator.

@fenfeng9

fenfeng9 commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

By the way, there seems to be another sparse union-related bug. I'll take a closer look tomorrow, and if I can confirm it, I'll open a separate issue.

@fenfeng9

fenfeng9 commented Jun 6, 2026

Copy link
Copy Markdown
Contributor Author

Another related issue: #50113

@fenfeng9

fenfeng9 commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

@rok Could you please review this pr when you have time?

@pitrou pitrou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you @fenfeng9

@pitrou pitrou merged commit f8184d5 into apache:main Jun 10, 2026
56 checks passed
@pitrou pitrou removed the awaiting review Awaiting review label Jun 10, 2026
@github-actions github-actions Bot added the awaiting committer review Awaiting committer review label Jun 10, 2026
@rok

rok commented Jun 10, 2026

Copy link
Copy Markdown
Member

Thanks @fenfeng9!

@conbench-apache-arrow

Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit f8184d5.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 4 possible false positives for unstable benchmarks that are known to sometimes produce them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants